Add func-operator integration to deploy command by creydr · Pull Request #3657 · knative/func

creydr · 2026-05-05T12:21:45Z

Changes

🎁 Create a Function CR on func deploy when func-operator is installed
🎁 Add option to skip Function CR creation via func deploy --management-disabled
🔧 Bump Go to 1.26 (required by controller-runtime dependency)

/kind enhancement

Release Note

Create Function CR on `func deploy`

knative-prow · 2026-05-05T12:21:52Z

[APPROVALNOTIFIER] This PR is NOT APPROVED

This pull-request has been approved by: creydr
Once this PR has been reviewed and has the lgtm label, please assign dprotaso for approval. For more information see the Code Review Process.

The full list of commands accepted by this bot can be found here.

Details

Needs approval from an approver in each of these files:

OWNERS

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

codecov · 2026-05-05T13:00:02Z

Codecov Report

❌ Patch coverage is 45.50409% with 200 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.70%. Comparing base (03a7c64) to head (470d603).

Files with missing lines	Patch %	Lines
pkg/deployer/testing/integration_test_helper.go	0.00%	131 Missing ⚠️
pkg/operator/sync.go	68.93%	22 Missing and 10 partials ⚠️
pkg/git/resolver.go	67.69%	14 Missing and 7 partials ⚠️
pkg/operator/syncer.go	74.50%	10 Missing and 3 partials ⚠️
pkg/functions/client.go	62.50%	2 Missing and 1 partial ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3657      +/-   ##
==========================================
- Coverage   56.91%   56.70%   -0.21%     
==========================================
  Files         181      184       +3     
  Lines       20928    21291     +363     
==========================================
+ Hits        11911    12074     +163     
- Misses       7810     7988     +178     
- Partials     1207     1229      +22

Flag	Coverage Δ
e2e	`36.13% <35.16%> (+<0.01%)`	⬆️
e2e go	`32.80% <38.42%> (+0.04%)`	⬆️
e2e node	`28.59% <38.42%> (+0.10%)`	⬆️
e2e python	`33.16% <38.42%> (+0.04%)`	⬆️
e2e quarkus	`28.73% <38.42%> (+0.12%)`	⬆️
e2e rust	`28.14% <38.42%> (+0.11%)`	⬆️
e2e springboot	`26.66% <38.42%> (+0.15%)`	⬆️
e2e typescript	`28.69% <38.42%> (+0.10%)`	⬆️
e2e-config-ci	`17.75% <9.47%> (-0.13%)`	⬇️
integration	`17.14% <0.00%> (-0.32%)`	⬇️
unit macos-14	`44.78% <34.12%> (-0.21%)`	⬇️
unit macos-latest	`44.78% <34.12%> (-0.21%)`	⬇️
unit ubuntu-24.04-arm	`44.92% <32.15%> (-0.24%)`	⬇️
unit ubuntu-latest	`45.61% <34.12%> (-0.23%)`	⬇️
unit windows-latest	`44.76% <34.12%> (-0.22%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

lkingland

Looks great! My only substantive suggestion is that this be integrated more deeply into core instead of an ad-hoc post-deploy hook 👍🏻

lkingland · 2026-05-08T05:12:55Z

 module knative.dev/func

-go 1.25.9
+go 1.26


I presume this was necessary due to controller-runtime dependency? Maybe just add to the commit description this PR also bumps to Go 1.26

Yes, required by controller-runtime. Added a note to the PR description.

lkingland · 2026-05-08T05:12:56Z

+	}
+
+	if cfg.RepoURL == "" {
+		fmt.Fprintln(os.Stderr, "Function CR not created: no git remote found. Set --git-url to enable operator management.")


This warning will fire on every deploy without a configured git remote, given --manage=true is the default. I think that there are actually two preconditions for a Function to be "managed": The operator is installed AND they're using git.

The base case: A user just initializes a function and deployes it directly to a vanilla cluster, shouldn't really even print a warning (it's an expected base case).

Good point. Changed to skip silently when no git remote is configured — the two preconditions are now operator CRD installed AND git URL available.

lkingland · 2026-05-08T05:12:56Z

+	}
+
+	if existing != nil {
+		existing.Spec = desiredSpec


Just double-checking: without using a merge, this will always overwrite the spec, even if changed in some different way. I think that's what we want (again encouraging declarative config), but just want to flag this to be sure.

Correct, this is intentional. The spec is always overwritten with the local state to encourage declarative config.

lkingland · 2026-05-08T05:12:56Z

+func hasFunctionCRD(disc discovery.DiscoveryInterface) (bool, error) {
+	resources, err := disc.ServerResourcesForGroupVersion("functions.dev/v1alpha1")
+	if err != nil {
+		return false, nil


This swallows any error from ServerResourcesForGroupVersion and treats it identically to "CRD not present." If there's a legitimate reason, we probably want to raise an error. Perhaps something like:

resources, err := disc.ServerResourcesForGroupVersion("functions.dev/v1alpha1") if err != nil { if apierrors.IsNotFound(err) || meta.IsNoMatchError(err) { return false, nil } return false, fmt.Errorf("checking for Function CRD: %w", err) }

(I haven't fully traced the call path here; just an example)

Fixed. Now only IsNotFound errors are treated as CRD absence; other errors are propagated.

lkingland · 2026-05-08T05:12:56Z

+		if list.Items[i].Status.Name == funcName {
+			return &list.Items[i], nil
+		}
+	}
+
+	for i := range list.Items {
+		if list.Items[i].Name == funcName {


Minor nit: these two loops over list.Items made me double-check to see if it was an error. maybe just put both if statements in one for-loop for clarity that this was intended.

Combined into a single loop. Status name match still takes priority (returns immediately); metadata name match is remembered as fallback.

lkingland · 2026-05-08T05:23:28Z

+	if c.Manage {
+		o = append(o, fn.WithPostDeploy(func(ctx context.Context, f fn.Function) error {
+			if err := syncFunctionCR(ctx, f, c, creds); err != nil {
+				fmt.Fprintf(os.Stderr, "Warning: failed to sync Function CR: %v\n", err)
+			}
+			return nil
+		}))
+	}


This should probably be part of core rather than in the CLI. Adding a boolean to the Function structh which states this is a "managed" Function, and a CR should be created (if the operator is installed).

Done. Moved into core Client.Deploy() via a FunctionSyncer interface, with ManagementDisabled field on DeploySpec (managed by default).

lkingland · 2026-05-08T05:26:12Z

 	pipelinesProvider PipelinesProvider // CI/CD pipelines management
 	mcpServer         MCPServer         // MCP Server
 	startTimeout      time.Duration     // default start timeout for all runs
+	postDeploy        func(context.Context, Function) error


No need for a post-deploy hook if we just extend the core's "deploy" logic to do the CR work if the given function is a "managed" function.

To make it even simpler, the flag could be "ManagementDisabled", such that the deafult will always be to create the CR if the system detects the operator is installed.

Done. Replaced postDeploy hook with FunctionSyncer interface. --manage flag replaced by --management-disabled.

When deploying a function, automatically create or update a Function CR (functions.dev/v1alpha1) if the func-operator CRD is installed on the cluster. On first deploy, the CR is created with the git repository URL, branch, and path. On subsequent deploys, only the functions.knative.dev/last-deployed annotation is updated. - Add pkg/git/ with go-git based resolution of remote URL and branch - Add pkg/operator/ with Function CR sync logic and k8s client setup - Add WithPostDeploy client option to run post-deploy hooks - Add --manage flag (default true) to opt out with --manage=false - Wire operator sync via WithPostDeploy in deploy command - Git URL cascade: --git-url > tracking remote > origin - Branch cascade: --git-branch > current branch > "main" - Silently skip if CRD not installed; warn if no git remote found

Add TestInt_OperatorSync to the shared deployer integration test helper. The test deploys a function with WithPostDeploy wired to operator.SyncFunctionCR, verifies the Function CR is created with the correct spec on first deploy, and confirms only the last-deployed annotation is updated on subsequent deploys. CR verification is skipped gracefully when the Function CRD is not installed.

Add func-operator v0.2.1 to the cluster setup script so the Function CRD is available for integration tests. The operator is installed in parallel with other components during cluster allocation.

ResolveRemoteURL now removes userinfo (username and password) from HTTP/HTTPS remote URLs before returning them, preventing credentials from being stored in the Function CR spec.

When deploying to a private registry, resolve credentials via the existing CredentialsProvider, create a docker-registry K8s Secret, and set .spec.registry.authSecretRef on the Function CR so the func-operator can authenticate with the registry.

- Check return value of AddToScheme (errcheck lint) - Make registry secret creation injectable for unit tests - Add git commit author in resolver tests for CI - Add FuncOperator to component-versions test expectations

creydr · 2026-05-11T09:13:46Z

rebased and resolved merge conflicts

Replace the CLI-level postDeploy hook with a FunctionSyncer interface in pkg/functions, implemented by pkg/operator.Syncer. This makes operator management available to all library users, not just the CLI. - Add FunctionSyncer interface and WithSyncer option to Client - Add ManagementDisabled field to DeploySpec (managed by default) - Create pkg/operator/syncer.go encapsulating git/registry resolution - Wire operator.NewSyncer in cmd/client.go NewClient() - Replace --manage flag with --management-disabled - Remove postDeploy hook, syncFunctionCR from cmd/deploy.go - Update integration test to use WithSyncer

A user deploying without a git remote is an expected base case, not a warning-worthy condition. The operator sync now requires both the CRD to be installed and a git URL to be available before attempting to create or update a Function CR.

Previously all errors from ServerResourcesForGroupVersion were swallowed and treated as "CRD not present". Now only NotFound errors are treated as CRD absence; other errors are propagated so legitimate failures are not silently ignored.

Merge the separate status-name and metadata-name lookups into a single pass over the list. Status name match still takes priority and returns immediately; metadata name match is remembered as a fallback.

Replace the hardcoded "functions.dev/v1alpha1" string with v1alpha1.GroupVersion.String() in hasFunctionCRD, unit tests, and integration test helper.

Update generated docs to reflect --manage being replaced by --management-disabled.

Add ManagementDisabled field to the generated schema.

creydr · 2026-05-11T11:30:37Z

Thanks for the review @lkingland
I addressed your review (hopefully :D )

Only maybe one thing we could also address in this PR: WDYT about removing the CR when --management-disabled is set?

knative-prow Bot added do-not-merge/work-in-progress 🤖 PR should not merge because it is a work in progress. kind/enhancement Feature additions or improvements to existing labels May 5, 2026

knative-prow Bot requested review from dsimansk and jrangelramos May 5, 2026 12:21

knative-prow Bot added the size/XXL 🤖 PR changes 1000+ lines, ignoring generated files. label May 5, 2026

knative-prow-robot added the needs-rebase Cannot be merged due to conflicts with HEAD. label May 5, 2026

creydr force-pushed the create-func-crd-on-deploy-2 branch from 6bd73c4 to ba2e1c9 Compare May 5, 2026 13:21

knative-prow-robot added needs-rebase Cannot be merged due to conflicts with HEAD. and removed needs-rebase Cannot be merged due to conflicts with HEAD. labels May 5, 2026

lkingland reviewed May 8, 2026

View reviewed changes

creydr added 6 commits May 11, 2026 11:02

Install func-operator in test cluster setup

7dd12e8

Add func-operator v0.2.1 to the cluster setup script so the Function CRD is available for integration tests. The operator is installed in parallel with other components during cluster allocation.

Strip credentials from git remote URLs in Function CR

b3b2cbd

ResolveRemoteURL now removes userinfo (username and password) from HTTP/HTTPS remote URLs before returning them, preventing credentials from being stored in the Function CR spec.

Fix CI failures in linter and unit tests

38650ee

- Check return value of AddToScheme (errcheck lint) - Make registry secret creation injectable for unit tests - Add git commit author in resolver tests for CI - Add FuncOperator to component-versions test expectations

creydr force-pushed the create-func-crd-on-deploy-2 branch from ba2e1c9 to 38650ee Compare May 11, 2026 09:13

knative-prow-robot removed the needs-rebase Cannot be merged due to conflicts with HEAD. label May 11, 2026

creydr added 7 commits May 11, 2026 11:54

Combine two loops in findExistingCR into one

278886e

Merge the separate status-name and metadata-name lookups into a single pass over the list. Status name match still takes priority and returns immediately; metadata name match is remembered as a fallback.

Use func-operator GroupVersion constant instead of hardcoded string

9f0bda8

Replace the hardcoded "functions.dev/v1alpha1" string with v1alpha1.GroupVersion.String() in hasFunctionCRD, unit tests, and integration test helper.

Regenerate deploy command reference docs

af3f6be

Update generated docs to reflect --manage being replaced by --management-disabled.

Regenerate func.yaml JSON schema

470d603

Add ManagementDisabled field to the generated schema.

creydr changed the title ~~[WIP] Add func-operator integration to deploy command~~ Add func-operator integration to deploy command May 11, 2026

knative-prow Bot removed the do-not-merge/work-in-progress 🤖 PR should not merge because it is a work in progress. label May 11, 2026

creydr requested a review from lkingland May 11, 2026 11:29

Conversation

creydr commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Uh oh!

knative-prow Bot commented May 5, 2026

Uh oh!

codecov Bot commented May 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

lkingland left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

creydr commented May 11, 2026

Uh oh!

creydr commented May 11, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

creydr commented May 5, 2026 •

edited

Loading

codecov Bot commented May 5, 2026 •

edited

Loading